fix(worker): preserve permissions on transient token refresh failures by brendan-kellam · Pull Request #1481 · sourcebot-dev/sourcebot

brendan-kellam · 2026-07-22T16:59:54Z

Classify OAuth token refresh failures so transient provider outages are retried without clearing cached permissions, while invalid_grant remains fail-closed.

Fixes SOU-1560

Fixes SOU-1177

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

Note

Medium Risk
Changes when cached repository permissions are cleared vs retained during permission sync failures, which directly affects EE access control until the next successful sync.

Overview
Fixes account permission sync wiping cached repo permissions whenever token refresh failed. Refresh errors are now classified instead of treating every refresh failure as fail-closed.

Token refresh (tokenRefresh.ts) throws typed TokenRefreshError values (e.g. invalid_grant / GitHub bad_refresh_token → refresh_token_rejected; 5xx, timeouts, server_error → transient with up to 3 retries). Failures no longer collapse to a generic error that always triggered permission cleanup.

Permission sync (accountPermissionSyncer.ts) uses classifyPermissionSyncFailure: clear permissions only for rejected refresh tokens or code-host API 401/403/410; preserve permissions for transient outages, misconfiguration, and other non-auth failures (including token-endpoint 401 that is classified as configuration, not API unauthorized).

Vitest coverage was added for classification and refresh behavior; CHANGELOG notes the EE fix.

^{Reviewed by Cursor Bugbot for commit 27d517b. Bugbot is set up for automated code reviews on this repo. Configure here.}

Summary by CodeRabbit

Bug Fixes
- Preserve existing repository permissions when OAuth token refresh fails due to transient or non-authorization issues, including refresh token rejections.
- Clear permissions only for definitive authorization/credential failures (OAuth invalid_grant and HTTP 401/403/410), using consistent failure classification.
- Store and reuse structured token refresh failure information to improve downstream cleanup decisions and logging.
Tests
- Added Vitest suites covering token refresh retry behavior and fail-closed vs preserve-closed permission-sync classification.

coderabbitai · 2026-07-22T17:00:19Z

Walkthrough

OAuth token refresh now uses typed failures, bounded retries, timeout and response classification. Account permission synchronization uses those classifications to clear permissions only for authorization failures and preserve them for transient or unrelated failures.

Changes

OAuth refresh and exchange flow

Layer / File(s)	Summary
Typed refresh errors and credential handling `packages/backend/src/ee/tokenRefresh.ts`	Adds structured `TokenRefreshError` classifications for credential, configuration, and provider setup failures, replacing nullable failure returns.
OAuth exchange retries and validation `packages/backend/src/ee/tokenRefresh.ts`, `packages/backend/src/ee/tokenRefresh.test.ts`	Adds bounded retries, timeout handling, OAuth error parsing, successful-response validation, and coverage for retry and non-retry cases.

Permission synchronization

Layer / File(s)	Summary
Permission cleanup decisions `packages/backend/src/ee/accountPermissionSyncer.ts`, `packages/backend/src/ee/accountPermissionSyncer.test.ts`, `CHANGELOG.md`	Maps OAuth refresh-token rejection and HTTP 401/403/410 failures to permission cleanup, preserves permissions for other failures, and documents the behavior.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant AccountPermissionSyncer
  participant ensureFreshAccountToken
  participant exchangeRefreshToken
  participant OAuthProvider
  AccountPermissionSyncer->>ensureFreshAccountToken: refresh account token
  ensureFreshAccountToken->>exchangeRefreshToken: exchange refresh token
  exchangeRefreshToken->>OAuthProvider: POST OAuth token request
  OAuthProvider-->>exchangeRefreshToken: token or classified error
  exchangeRefreshToken-->>ensureFreshAccountToken: token or TokenRefreshError
  ensureFreshAccountToken-->>AccountPermissionSyncer: success or failure
  AccountPermissionSyncer->>AccountPermissionSyncer: classify cleanup decision

Possibly related PRs

sourcebot-dev/sourcebot#1000: Both modify the token refresh flow and its use by account permission synchronization.
sourcebot-dev/sourcebot#1215: Both update account permission synchronization handling for authorization and token refresh failures.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main user-visible change: permissions are preserved when token refresh fails transiently.

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch brendan/fix-token-refresh-permission-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

cursor

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 0954bf8. Configure here.}

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/backend/src/ee/tokenRefresh.ts`:
- Around line 307-343: The retry loop around the token refresh fetch must not
replay refresh grants once the request may have reached the provider. Update the
error classification/control flow near classifyTokenRefreshFetchError and the
retry condition to retry only failures known to occur before request
transmission; propagate timeout, connection, or response-uncertainty errors
without retrying, especially for gitlab and bitbucket-cloud. Preserve retries
for definitively pre-request failures and the existing final-error behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a60b0bd5-6143-4051-b1d3-8b4f79064f11

📥 Commits

Reviewing files that changed from the base of the PR and between e1ce46b and 0954bf8.

📒 Files selected for processing (5)

CHANGELOG.md
packages/backend/src/ee/accountPermissionSyncer.test.ts
packages/backend/src/ee/accountPermissionSyncer.ts
packages/backend/src/ee/tokenRefresh.test.ts
packages/backend/src/ee/tokenRefresh.ts

Classify OAuth token refresh failures so transient provider outages are retried without clearing cached permissions, while invalid_grant remains fail-closed. Fixes SOU-1560 Fixes SOU-1177

coderabbitai

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Around line 16-17: Remove the duplicate “### Fixed” heading in CHANGELOG.md
and place the OAuth token refresh bullet under the existing Fixed section,
preserving the bullet text and ordering.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8dfa707a-8015-4311-aa13-c4681ea98667

📥 Commits

Reviewing files that changed from the base of the PR and between 0954bf8 and 4d6cdd0.

📒 Files selected for processing (5)

CHANGELOG.md
packages/backend/src/ee/accountPermissionSyncer.test.ts
packages/backend/src/ee/accountPermissionSyncer.ts
packages/backend/src/ee/tokenRefresh.test.ts
packages/backend/src/ee/tokenRefresh.ts

This comment has been minimized.

Sign in to view

cursor Bot reviewed Jul 22, 2026

View reviewed changes

Comment thread packages/backend/src/ee/tokenRefresh.ts

coderabbitai Bot reviewed Jul 22, 2026

View reviewed changes

Comment thread packages/backend/src/ee/tokenRefresh.ts Outdated

This was referenced Jul 22, 2026

fix: surface permission sync issues requiring user action #1484

Merged

fix(worker): classify permission sync failures by provider context #1482

Merged

brendan-kellam added 3 commits July 23, 2026 14:00

fix(worker): preserve permissions on transient token refresh failures

bbc3b7f

Classify OAuth token refresh failures so transient provider outages are retried without clearing cached permissions, while invalid_grant remains fail-closed. Fixes SOU-1560 Fixes SOU-1177

chore: add changelog entry for token refresh fix

e6e530b

Handle OAuth errors returned with successful HTTP status

4d6cdd0

brendan-kellam force-pushed the brendan/fix-token-refresh-permission-cleanup branch from 0954bf8 to 4d6cdd0 Compare July 23, 2026 18:02

coderabbitai Bot reviewed Jul 23, 2026

View reviewed changes

Comment thread CHANGELOG.md Outdated

Fix unreleased changelog section ordering

27d517b

jsourcebot approved these changes Jul 23, 2026

View reviewed changes

brendan-kellam merged commit 60ca4e4 into main Jul 23, 2026
11 checks passed

github-actions Bot mentioned this pull request Jul 23, 2026

Sourcebot Roadmap 🚀 #459

Open

brendan-kellam deleted the brendan/fix-token-refresh-permission-cleanup branch July 23, 2026 20:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(worker): preserve permissions on transient token refresh failures#1481

fix(worker): preserve permissions on transient token refresh failures#1481
brendan-kellam merged 4 commits into
mainfrom
brendan/fix-token-refresh-permission-cleanup

brendan-kellam commented Jul 22, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

This comment has been minimized.

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

brendan-kellam commented Jul 22, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

This comment has been minimized.

coderabbitai Bot commented Jul 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

brendan-kellam commented Jul 22, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 22, 2026 •

edited

Loading